Add new test case for container based warm restart #6054
Conversation
Change-Id: Ic30e2a2d6d567ea1ee2869e0c07e394f1524c326
Change-Id: I7721bcf1e72bd95f866226860b80b17c33380063
Change-Id: I34383bdf4ce7b658c71a9488ab077e787f6cccd0
|
This pull request introduces 4 alerts when merging 4c8f3b0 into c7d1948 - view on LGTM.com new alerts:
|
|
/easycla |
|
@stepanblyschak , @yxieca please help to review |
|
@yxieca could you please help to review or assing someone? |
|
Ack. I am reviewing this PR now. |
| @@ -658,7 +727,13 @@ def tearDown(self): | |||
| self.__runScript([self.postRebootCheckScript], self.duthost) | |||
|
|
|||
| if not self.stayInTargetImage: | |||
| self.__restorePrevImage() | |||
| logger.info('Restoring previous image') | |||
There was a problem hiding this comment.
stay_in_target_image arg is by default set to true. I think it would be better to always revert the device back to pretest state. Otherwise it can impact subsequent testcases that might run in long running regression tests.
There was a problem hiding this comment.
stay_in_target_image is a flag specified by user. I agree with you that the default value should be false, however, I am not sure if we should change the default behavior here. Because the test case is already running on regression system, vendors may not expect to have the default behavior changed.
| self.dut_connection.execCommand('docker exec -i bgp pkill -9 zebra') | ||
| self.dut_connection.execCommand('docker exec -i bgp pkill -9 bgpd') | ||
| elif service_name == 'teamd': | ||
| self.dut_connection.execCommand('docker exec -i teamd pkill -USR1 teamd > /dev/null') |
There was a problem hiding this comment.
What about syncd warm restart? In which case, there needs to be syncd preshutdown?
There was a problem hiding this comment.
As far as I know, syncd warm restart is not supported.
Change-Id: I7b2ef8f6cd249b7c870fc5d9fc5fbe80bbb41e01
|
This pull request introduces 1 alert when merging 99a87c3 into 2e40f50 - view on LGTM.com new alerts:
|
|
/azpw run Azure.sonic-mgmt |
|
/AzurePipelines run Azure.sonic-mgmt |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azpw run Azure.sonic-mgmt |
|
/AzurePipelines run Azure.sonic-mgmt |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azpw run Azure.sonic-mgmt |
|
/AzurePipelines run Azure.sonic-mgmt |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Hi @vaibhavhd, could you please review and sign-off? |
|
@vaibhavhd any further open issues or can you approve? |
Added a new test case test_advanced_reboot::test_service_warm_restart. This test tries to run warm restart for each service in FEATURE table. If a URL is provided by CLI option "--new_docker_image", the test case will try download new docker image from the URL and do in service warm upgrade. The new test case reused the advanced-reboot infrastructure in sonic-mgmt and added some special logic for container based warm restart
Description of PR
Summary:
Add new test case for container based warm restart
Type of change
Back port request
Approach
What is the motivation for this PR?
Add new test case for container based warm restart
How did you do it?
Added a new test case test_advanced_reboot::test_service_warm_restart.
How did you verify/test it?
Run the test case
Any platform specific information?
N/A
Supported testbed topology if it's a new test case?
t0
Documentation